Skip to content

feat(apollo-usage-report): report referenced feild by types #4020

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

feat(apollo-usage-report): report referenced feild by types #4020

wants to merge 6 commits into from

Conversation

EmrysMyrddin
Copy link
Collaborator

@EmrysMyrddin EmrysMyrddin commented May 5, 2025 •
edited
Loading

Related to YOGA-19
Related to #3580

@codesandbox CodeSandbox
Copy link

codesandbox bot commented May 5, 2025

Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders

Open Preview

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 5, 2025 •
edited
Loading

Apollo Federation Subgraph Compatibility Results

Federation 1 Support Federation 2 Support
_service🟢
@key (single)🟢
@key (multi)🟢
@key (composite)🟢
repeatable @key🟢
@requires🟢
@provides🟢
federated tracing🟢
@link🟢
@shareable🟢
@tag🟢
@override🟢
@inaccessible🟢
@composeDirective🟢
@interfaceObject🟢

Learn more:

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 5, 2025 •
edited
Loading

💻 Website Preview

The latest changes are available as preview in: https://pr-4020.graphql-yoga.pages.dev

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 5, 2025 •
edited
Loading

✅ Benchmark Results

     ✓ no_errors{mode:graphql}
     ✓ expected_result{mode:graphql}
     ✓ no_errors{mode:graphql-jit}
     ✓ expected_result{mode:graphql-jit}
     ✓ no_errors{mode:graphql-response-cache}
     ✓ expected_result{mode:graphql-response-cache}
     ✓ no_errors{mode:graphql-no-parse-validate-cache}
     ✓ expected_result{mode:graphql-no-parse-validate-cache}
     ✓ no_errors{mode:uws}
     ✓ expected_result{mode:uws}

     checks.......................................: 100.00% ✓ 412728      ✗ 0     
     data_received................................: 1.7 GB  11 MB/s
     data_sent....................................: 83 MB   552 kB/s
     http_req_blocked.............................: avg=1.61µs   min=992ns    med=1.39µs   max=276.46µs p(90)=2.1µs    p(95)=2.34µs  
     http_req_connecting..........................: avg=3ns      min=0s       med=0s       max=158.72µs p(90)=0s       p(95)=0s      
     http_req_duration............................: avg=502.49µs min=276.46µs med=462.33µs max=19ms     p(90)=647.43µs p(95)=674.94µs
       { expected_response:true }.................: avg=502.49µs min=276.46µs med=462.33µs max=19ms     p(90)=647.43µs p(95)=674.94µs
     ✓ { mode:graphql-jit }.......................: avg=461.61µs min=358.32µs med=420.71µs max=19ms     p(90)=465.85µs p(95)=493.85µs
     ✓ { mode:graphql-no-parse-validate-cache }...: avg=699.05µs min=563.62µs med=649.19µs max=9.78ms   p(90)=702.41µs p(95)=781.09µs
     ✓ { mode:graphql-response-cache }............: avg=525.77µs min=408.85µs med=483.56µs max=12.79ms  p(90)=527.17µs p(95)=551.4µs 
     ✓ { mode:graphql }...........................: avg=549.78µs min=412.02µs med=495.12µs max=18.64ms  p(90)=570.79µs p(95)=653.66µs
     ✓ { mode:uws }...............................: avg=360.74µs min=276.46µs med=339.79µs max=7.7ms    p(90)=375.89µs p(95)=394.34µs
     http_req_failed..............................: 0.00%   ✓ 0           ✗ 206364
     http_req_receiving...........................: avg=34.95µs  min=16.23µs  med=34.04µs  max=2.89ms   p(90)=41.59µs  p(95)=45.82µs 
     http_req_sending.............................: avg=9.23µs   min=6.1µs    med=8.25µs   max=2.33ms   p(90)=11.93µs  p(95)=13.42µs 
     http_req_tls_handshaking.....................: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting.............................: avg=458.3µs  min=248.35µs med=418.82µs max=18.89ms  p(90)=602.49µs p(95)=627.93µs
     http_reqs....................................: 206364  1375.743575/s
     iteration_duration...........................: avg=721.73µs min=462.11µs med=676.34µs max=19.56ms  p(90)=869.17µs p(95)=903.02µs
     iterations...................................: 206364  1375.743575/s
     vus..........................................: 1       min=1         max=1   
     vus_max......................................: 2       min=2         max=2

@EmrysMyrddin EmrysMyrddin force-pushed the feat/apollo-usage-report/per-field-count branch from 418b0f5 to ee4b71e Compare May 6, 2025 09:15
@EmrysMyrddin EmrysMyrddin force-pushed the feat/apollo-usage-report/per-field-count branch from ee4b71e to 48495ac Compare May 6, 2025 09:21
@EmrysMyrddin EmrysMyrddin mentioned this pull request May 9, 2025
Open
ardatan
ardatan reviewed May 10, 2025
View reviewed changes
packages/plugins/apollo-usage-report/src/stats.ts
// Copied from https://github.com/apollographql/apollo-server/blob/8c6579e5b61276b62dc7e30e6fac9a4242e24daa/packages/server/src/plugin/usageReporting/stats.ts
/* eslint-disable */

import type { NonFtv1ErrorPath } from '@apollo/server-gateway-interface';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can avoid this package entirely by copying this too, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can. We discussed it on slack, and we decided that if the Licence allows it, and the dependency is small, it is ok to have it as a dependence.

That's why I've made a copy of the relevant part of the apollo-server code. I don't think it is worth to embed the whole apollo-server deps just for a class ^^'

ardatan
ardatan reviewed May 10, 2025
View reviewed changes
packages/plugins/apollo-usage-report/src/index.ts
hostname?: string;
/**
* The OS identification string.
* The format is `${os.platform()}, ${os.type()}, ${os.release()}, ${os.arch()})`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

navigator.platform etc can be used in platform independent way, maybe?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I didn't know node had navigator API.
The thing I don't know is if Apollo is parsing this field or not. If it's paring the string, then we can't use navigator.platform.

@EmrysMyrddin EmrysMyrddin force-pushed the feat/apollo-usage-report/per-field-count branch from 3e7a4d0 to 4e78157 Compare May 20, 2025 09:28
@github-actions GitHub Actions
Copy link
Contributor

🚀 Snapshot Release (alpha)

The latest changes of this PR are available as alpha on npm (based on the declared changesets):

Package Version Info
@graphql-yoga/plugin-apollo-usage-report 0.9.0-alpha-20250520092918-4e78157511f024b73e7df2eb0797de0242e81e34 npm ↗︎ unpkg ↗︎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kroupacz kroupacz kroupacz left review comments

@ardatan ardatan ardatan left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@EmrysMyrddin @kroupacz @ardatan @enisdenjo